Skip to content

Conversation

juankx-bodo
Copy link
Contributor

@juankx-bodo juankx-bodo commented Sep 9, 2025

Improve the error msg name must be a string that is a Python identifier to:
name: [no!Valid'Identifier\Name] must be a string that is a valid Python identifier

Add additional validation to names used in Pydough to prevent them from being Pydough or Python reserved words. Also, add validation for table path and column name to ensure they are valid SQL identifier names and not SQL reserved words.

@knassre-bodo
Copy link
Contributor

@juankx-bodo You need to run CI

@juankx-bodo juankx-bodo force-pushed the jkx/Improve_metadata_error_message branch from bf81cfe to b127491 Compare September 10, 2025 21:24
Copy link
Contributor

@john-sanchez31 john-sanchez31 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good

Copy link
Contributor

@hadia206 hadia206 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thanks Juan

Extend with new PyDough reserved words if required.
"""
PYDOUGH_RESERVED: set[str] = {
"CALCULATE",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other thing we should probably ban is any of the function names (see the builtin_registered_operators function for how to obtain these names).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done! I didn't know about builtin_registered_operators

@juankx-bodo juankx-bodo force-pushed the jkx/Improve_metadata_error_message branch 2 times, most recently from 9565709 to e52c486 Compare September 25, 2025 05:32
Improve metadata error messages
Improve metadata validations
Check for reserved python keywords on metadata names
Fix metadata for broker
Check for Pydough and SQL reserved words
Add validation for SQL names in TablePath & ColumnName
Apply code review observations
@juankx-bodo juankx-bodo force-pushed the jkx/Improve_metadata_error_message branch from cea91f0 to d96e49f Compare September 30, 2025 15:12
@juankx-bodo juankx-bodo merged commit 6675570 into main Sep 30, 2025
10 checks passed
@juankx-bodo juankx-bodo deleted the jkx/Improve_metadata_error_message branch September 30, 2025 15:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants